Skip to content

Use synchronous runtime for the main wasm execution lane#5095

Open
joshua-spacetime wants to merge 1 commit into
masterfrom
joshua/wasm-sync
Open

Use synchronous runtime for the main wasm execution lane#5095
joshua-spacetime wants to merge 1 commit into
masterfrom
joshua/wasm-sync

Conversation

@joshua-spacetime
Copy link
Copy Markdown
Contributor

Description of Changes

Before this change, we used a single async-enabled wasm runtime for all requests, even though procedures are the only operation that can yield. Now each module gets two separate runtimes. We continue to use the same async runtime for procedures, but now reducers are executed against a synchronous wasm runtime, backed by a single OS-thread instead of a Tokio runtime.

The purpose of this change is to remove from the critical path the overhead associated with async calls that really aren't async at all.

API and ABI breaking changes

None

Expected complexity level and risk

2.5

Testing

Pure refactor. Relies on current test coverage. #5078 will ensure the performance is on par with V8.

Comment on lines +139 to 151
fn wasm_main_worker_thread_name(database_identity: &spacetimedb_lib::Identity) -> String {
let hex = database_identity.to_hex();
// We use the tail of the identity to avoid the common structured prefix.
let suffix = &hex.as_str()[hex.as_str().len() - THREAD_NAME_DATABASE_ID_SUFFIX_LEN..];
format!("wasm-main-{suffix}")
}

fn wasm_procedure_executor_thread_name(database_identity: &spacetimedb_lib::Identity) -> String {
let hex = database_identity.to_hex();
// We use the tail of the identity to avoid the common structured prefix.
let suffix = &hex.as_str()[hex.as_str().len() - THREAD_NAME_DATABASE_ID_SUFFIX_LEN..];
format!("wasm-{suffix}")
format!("wasm-proc-{suffix}")
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Max length of a linux thread name is 15, so this will only show wasm-proc-XXXXX, even though suffix is XXXXXXXX

Comment on lines +713 to +719
if !self.supports_async {
let res = module_host_actor::ProcedureExecuteResult {
stats: zero_execution_stats(store),
call_result: Err(anyhow::anyhow!("procedures require an async Wasmtime instance")),
};
return (res, None);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might it make sense for this to be an assert!()? I think this would be a programmer error, not something a user could run into.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants